-
Notifications
You must be signed in to change notification settings - Fork 365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(command-env): add support for plaintext output in env:list #5322
Conversation
📊 Benchmark resultsComparing with 0bc18bb Package size: 249 MB(no change)
Legend
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thanks so much for pushing this along @tinfoil-knight!! 🙌
I have a request that shouldn't be too much lift: considering that we also have the undocumented --json
flag, and that --json --plain
doesn't make much sense, could this change to --format <format>
?
--format
would have an array of choices oftable
,plain
, andjson
, withtable
being the defaultoptions.json
should still be supported on line 71, alongsideoptions.format === 'json'
, to preserve backwards-compatibility
Let me know what you think. Thanks again for making this contribution! 💯
@@ -135,6 +135,7 @@ netlify env:list | |||
**Flags** | |||
|
|||
- `context` (*string*) - Specify a deploy context or branch (contexts: "production", "deploy-preview", "branch-deploy", "dev") | |||
- `plain` (*boolean*) - Output environment variables as plaintext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could new entries to the help be put below scope
? Would be nice to keep context and scope next to each other. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plain
flag in between seemed weird to me too but the options are sorted here to produced the documentation:
cli/src/utils/command-helpers.mjs
Lines 74 to 80 in d3dfa17
export const sortOptions = (optionA, optionB) => { | |
// base flags should be always at the bottom | |
if (BASE_FLAGS.has(optionA.long) || BASE_FLAGS.has(optionB.long)) { | |
return -1 | |
} | |
return optionA.long.localeCompare(optionB.long) | |
} |
Although, we can rename the option to text
so it appear after scope
.
Other alternatives like raw
, plaintext
would appear in between context
& scope
in the sorted order.
Let me know if you've a better suggestion or if we should go ahead with text
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha -- let's keep it as plain
, and let the docs sort them as they currently do. But in --help
, it would be great if context and scope could appear next to each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it seems like we're using the same function to order the help output too so I don't think the re-ordering would be possible unless we rename the option.
The current order does appear a bit weird to me too but I don't think there's any way to override this specific instance.
Ref:
cli/src/commands/base-command.mjs
Lines 249 to 256 in 3ff4a57
const optionList = helper | |
.visibleOptions(command) | |
.sort(sortOptions) | |
.map((option) => formatItem(helper.optionTerm(option), helper.optionDescription(option))) | |
if (optionList.length !== 0) { | |
output = [...output, chalk.bold('OPTIONS'), formatHelpList(optionList), ''] | |
} | |
} |
@jasonbarry I'm slightly opposed to making the suggested changes.
WDYT? P.S. I found a way to avoid the conflict with The commander CLI framework supports adding a conflicts property to the options to prevent conflicting options from being used together. $ ncli env:list --json --plain
› Error: option '--plain' cannot be used with option '--json'
› See more help with --help |
Sounds like a plan @tinfoil-knight, thanks for the research and explanation. I'd be happy to approve with this proposal if a |
@jasonbarry The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @tinfoil-knight, thanks for adding this feature!
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #3262
The
env:list
command currently outputs an ASCII table by default which isn't helpful if someone intends to write the environment variables to a file.The
--plain
flag introduced in this PR would allow a user to setup their env files easily on local systems since it outputs the environment variables in the plaintext format used in .env files.Example Output here:
Also see: #3262 (comment)
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)
🦥